BREAKING - analogWriteRange 8-bit default #7456
Conversation
Matching standard Arduino cores, make the default analogWrite() take values from 0...255. Users can always use the analogWriteRange() call to change to a different setup. Fixes esp8266#2895
There was a problem hiding this comment.
This isn't quite right.
- We need to have an implementation of analogWriteResolution(int bits), where bits goes from e.g. 4-10 or whatever, which maps to 0-15 - 0-1023 range. That doesn't mean remove the current analogWriteRange().
- analogWriteRange() needs to have the upper bound checked.
- The init of the default range needs to be changed to be 255 instead of 1023, that part is correct, but...
- We should either have:
- PWMRANGE represent our max capability and have a second define that is the default, e.g.: PWMRANGEDEFAULT
or - have PWMRANGE represent the default of 255 and have a second define that is the max of 1023, e.g.: PWMRANGEMAX
- PWMRANGE doesn't seem to exist in Arduino land, but that's ok
- do we need a res bits getter? I couldn't find anything looking at other core depots
Add a `analogWriteResolution` which takes a number of bits and sets the range from 0...(1<<bits)-1 Remove the PWMRANGE define. It's non-standard and not generally valid (i.e. it's fixed at 1024 of 256, but the real range varies depending on what you last set).
DONE
DONE
I suggest a different route: drop the macro. It doesn't track the current state of the machine and it's non-standard.
I say no. The number of bits of PWM resolution is not something that varies over time once set up. No good use case I can see for pulling it out. Would need same thing for range, and what happens when log2(range) has a fractional bit (i.e. |
There was a problem hiding this comment.
My one concern is that there is no way to tell whether the range/scale setting succeeded from the call itself (no return value), nor is there a getter to check the current value. That's certainly not a problem for cross-core code, because that functionality isn't part of the reference, but still.
I think the docs need to be updated with the min/max values accepted. I can handle that in a separate PR.
Approving.
Also add note about the change and how to fix pre 3.0 applications.
Matching standard Arduino cores, make the default analogWrite() take
values from 0...255. Users can always use the analogWriteRange() call
to change to a different setup.
Fixes #2895